Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

493 FilteredDatasetDefault class #497

Merged
merged 49 commits into from
Dec 14, 2023
Merged

Conversation

chlebowa
Copy link
Contributor

@chlebowa chlebowa commented Dec 12, 2023

Closes #493

Renamed class DefaultFilteredDataset to DataframeFilteredDataset.
Created new class DefaultFilteredDataset.
Renamed S3 methods for init_filteredDataset accordingly.

Removed assertion on dataset type in FilteredData$set_dataset.

New class DefaultFilteredDataset only has the following methods:

  • $initialize: initiates object
  • $format: returns string stating " - unfiltered dataset: : ", trims class to 37 chars by default
  • $get_call: returns NULL
  • $get_filter state: returns NULL
  • $set_filter_state: returns NULL with warning
  • $clear_filter_states: returns NULL
  • $get_filter_overview: returns NULL
  • $ui_active: displays empty div
  • $ui_add: displays empty div
  • $get_filter_overview: returns 1x3 data.frame with dataset name and two NAs in its only row

FilteredData$get_filter_overview puts the "all-NA" overviews at the bottom of the list.
FilteredData$srv_overview displays names of unfiltered datasets with warning icon that has a popover explaining that the data set in question is of an unsupported class.

Copy link
Contributor

github-actions bot commented Dec 12, 2023

Unit Tests Summary

    1 files    29 suites   22s ⏱️
365 tests 365 ✔️ 0 💤 0
841 runs  841 ✔️ 0 💤 0

Results for commit e748cbb.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried using it with a teal app (see below) and it fails due to not having implemented method ui_add on FilteredDatasetDefault

image

This works for me, although the message needs to be something else 😅

diff --git a/R/FilteredDatasetDefault.R b/R/FilteredDatasetDefault.R
index 5419cb45..0fced5dc 100644
--- a/R/FilteredDatasetDefault.R
+++ b/R/FilteredDatasetDefault.R
@@ -46,6 +46,21 @@ DefaultFilteredDataset <- R6::R6Class( # nolint
       sprintf(" - unfiltered dataset:\t\"%s\":   %s", private$dataname, class_string)
     },
 
+    #' @description
+    #' UI module to add filter variable for this dataset
+    #'
+    #' UI module to add filter variable for this dataset
+    #' @param id (`character(1)`)\cr
+    #'  identifier of the element - preferably containing dataset name
+    #'
+    #' @return function - shiny UI module
+    ui_add = function(id) {
+      ns <- NS(id)
+      tagList(
+        div("I'm not a real filter")
+      )
+    },
+

Below is the sample app

Expand sample code
library(teal)

td <- teal_data(
  new_iris = transform(iris, id = seq_len(nrow(iris))),
  code = "
      new_iris <- transform(iris, id = seq_len(nrow(iris)))
    "
) |> within({
  aa <- 1:10
})

datanames(td) <- c(datanames(td), "aa")

app <- init(
  data = td,
  modules = modules(
    example_module(label = "example teal module")
  ),
  title = "App title"
) |> shiny::runApp()

R/FilteredDatasetDefault.R Show resolved Hide resolved
R/FilteredData.R Outdated Show resolved Hide resolved
@chlebowa
Copy link
Contributor Author

That's quite a lot of warnings. Perhaps we should reconsider @gogonzo?

> devtools::load_all()
ℹ Loading teal.slice
> data(miniACC, package = "MultiAssayExperiment")
> 
> td <- teal.data::teal_data(
+   IRIS = iris,
+   CHARACTER = letters,
+   MAE = miniACC
+ )
> 
> app <- teal::init(td, teal::example_module())
> runApp(app)

Listening on http://127.0.0.1:3808
Warning in private$get_filtered_dataset(dataname)$set_filter_state(states) :
  DefaultFilterState cannot set state
Warning in x$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in x$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in x$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in x$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in x$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in x$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in self$get_filter_state() :
  DefaultFilterState does not have state to return
Warning in self$get_call(sid) :
  DefaultFilterState does not create filter calls
Warning in teal.data:::get_code_dependency(attr(datasets, "preprocessing_code"),  :
  Object(s) not found in code: IRIS, CHARACTER, MAE
Warning in private$get_filtered_dataset(dataname)$get_call() :
  DefaultFilterState does not create filter calls

@chlebowa chlebowa requested a review from averissimo December 12, 2023 16:38
@gogonzo gogonzo self-assigned this Dec 13, 2023
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would only throw a warning when trying to set filter.

R/FilteredDatasetDefault.R Outdated Show resolved Hide resolved
R/FilteredDatasetDefault.R Outdated Show resolved Hide resolved
R/FilteredDatasetDefault.R Outdated Show resolved Hide resolved
R/FilteredDatasetDefault.R Outdated Show resolved Hide resolved
R/FilteredDatasetDefault.R Outdated Show resolved Hide resolved
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😲

@chlebowa chlebowa enabled auto-merge (squash) December 14, 2023 14:09
@chlebowa chlebowa merged commit d753fe1 into main Dec 14, 2023
24 checks passed
@chlebowa chlebowa deleted the 493_FilteredDatasetDefault@main branch December 14, 2023 14:09
@gogonzo
Copy link
Contributor

gogonzo commented Dec 14, 2023

@pawelru @lcd2yyz @donyunardi @danielinteractive

It is official. teal is data-class independent. One can put data of any class and it will be transported to the module. Datasets unsupported by the FilteredData are displayed in the summary! Enjoy

options(teal.log_level = "TRACE", teal.show_js_log = TRUE, teal.bs_theme = bslib::bs_theme(version = 3))
library(shiny)
library(teal)

data <- teal_data() |>
  within({
    set.seed(1)
    library(scda)
    library(airway)
    library(MultiAssayExperiment)
    data(miniACC, envir = environment())
    data(airway, envir = environment())
    any_variable <- "elo"
    ADSL <- synthetic_cdisc_data("latest")$adsl
    ADSL$categorical <- sample(letters[1:3], size = nrow(ADSL), replace = TRUE, prob = c(.1, .3, .6))
    ADTTE <- synthetic_cdisc_data("latest")$adtte
    ADRS <- synthetic_cdisc_data("latest")$adrs
  })
datanames(data) <- c("ADSL", "ADTTE", "ADRS", "airway", "miniACC", "any_variable")
join_keys(data) <- default_cdisc_join_keys[c("ADSL", "ADTTE", "ADRS")]

default_filters <- teal.slice::teal_slices(
  teal_slice(dataname = "ADSL", varname = "categorical", selected = c("a"), id = "categorical", multiple = FALSE),
  teal_slice(id = "SE", title = "Safety-Evaluable", dataname = "ADSL", expr = "SAFFL == 'Y'"),
  teal_slice(dataname = "miniACC", varname = "years_to_birth", selected = c(30, 50), keep_na = TRUE, keep_inf = FALSE),
  teal_slice(dataname = "miniACC", varname = "gender", selected = "female", keep_na = TRUE),
  teal_slice(dataname = "miniACC", varname = "ARRAY_TYPE", selected = "protein_level", experiment = "RPPAArray", arg = "subset"),
  teal_slice(dataname = "airway", varname = "cell", selected = "N61311", experiment = "RNASeq"), # ignored
  teal_slice(dataname = "miniACC", varname = "Genes", selected = "G6PD", experiment = "RPPAArray", arg = "subset"),
  count_type = "none"
)

app <- init(
  data = data,
  modules = modules(
    modules(
      label = "tab1",
      example_module("funny"),
      example_module("funny", datanames = "airway"),
      example_module("funny2", datanames = "ADTTE"), # will limit datanames to ADTTE and ADSL (parent),
      teal.modules.general::tm_data_table(datasets_selected = c("ADSL", "ADTTE", "ADRS"))
    )
  ),
  filter = default_filters
)

runApp(app)
Skärmavbild 2023-12-14 kl  15 14 35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: FilteredDatasetDefault class for anything other than data.frame and MAE
4 participants